Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: npm install -g ttyper #97

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Anush008
Copy link

@Anush008 Anush008 commented Nov 5, 2023

This PR intends to add npm install -g ttyper as an installation strategy for Ttyper. Since NPM is the most used package manager, this would be a valuable addition to the project.

https://www.npmjs.com/package/ttyper

Here's how this works:

When npm install is run, NPM executes the install script, i.e. install.js.

The install.js file:

  • Downloads the appropriate binary from GitHub releases and moves it to the bin directory.
  • Removes the node_modules with the tar and zip libraries, as they are not needed any further.
  • NPM symlinks the package name ttyper, to whatever is set as the bin value in the package.json.
  • We configure bin to run runner.js, which handles the running of our downloaded binary.

The added npm-release Action job:

  • Bumps the version number value in the package.json to be in parity with the release version.
  • Copies the README.md from the root dir to be displayed on the package's homepage.
  • Publishes the package to NPM.

We can commit the version bump if we need to, but that would be an extra commit.

A successful run of this workflow can be found here.

@max-niederman, I'll move the NPM package to you and you can configure the NPM_ACCESS_TOKEN secret for this repository.

@Anush008
Copy link
Author

Hey @max-niederman, could you please review when you're available?
Thanks.

Copy link
Owner

@max-niederman max-niederman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, I just want to apologize for letting this go unreviewed for so long. I've been really busy lately with college applications and some health issues, but I should now have more time to maintain ttyper.

Anyway, I think there's some issue with this implementation. When I install the npm package and try to run ttyper, I get the following error:
image of error in Alpine container
As a minimal reproduction, you can get this error by starting an Alpine linux container, installing the npm package, then running npm install -g ttyper and ttyper. I'm not sure why this happens, exactly, but I suspect this is because it copies the runner file into /usr/local/bin, so trying to get the binary using a path relative to the runner script fails.

@Anush008
Copy link
Author

Anush008 commented Feb 2, 2024

Hi @max-niederman. Welcome back. I'll take a look.

Comment on lines +5 to +9
"repository": {
"type": "git",
"url": "git+https://github.com/Anush008/ttyper.git"
},
"releasesUrl": "https://github.com/Anush008/ttyper/releases",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Update these URLs to https://github.com/max-niederman/ttyper

@Anush008
Copy link
Author

Anush008 commented Mar 4, 2024

Hi @max-niederman. Could you try npm install -g ttyper and ttyper on your device now?

@Anush008 Anush008 requested a review from max-niederman March 25, 2024 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants